Fix problems in checking that a constructor is uninhabited for exhaustive match checking #23403
Conversation
Alex1005a
force-pushed
the
bottom-types-coverage-fix
branch
from
584b8e1 to
0696aec
Compare
There was a problem hiding this comment.
From my non-Space-engine-expert eyes this looks good to me.
Nice to see a PR that also removes some code!
I was also thinking about making the check recursive, i.e. checking that all fields are inhabited. This could cause problems with cycles and performance, so I'm not sure whether to do it.
Would the recursive check help in that situation for example?
sealed trait T
case class A(x: String) extends T
case class B(x: Nothing) extends T // uninhabited
sealed trait T2
case class A2(x: String) extends T2
case class B2(x: B) extends T2 // indirectly uninhabited
@main def Test =
(??? : T) match
case A(_) => ???
(??? : T2) match
case A2(_) => ??? // warn: match may not be exhaustive
// Do we want to remove this warning?Do we have real examples where that would be useful?
I’d merge this as-is and revisit deeper checks later if a concrete use-case turns up.
Yes, exactly.
Probably not. It might be useful for codebases that will heavily use this approach with Anyway, I think for now we can merge this as is. |
lazy, then the constructor is inhabited.Nothingand the field can be this type member.inhabitedfunction, allowing some changes to be reverted from Do not consider uninhabited constructors when performing exhaustive match checking #21750I was also thinking about making the check recursive, i.e. checking that all fields are inhabited. This could cause problems with cycles and performance, so I'm not sure whether to do it.